Add Electron start-up performance script #10442
Conversation
cdamus
force-pushed
the
electron-startup
branch
from
bbfad6c to
4ff4f95
Compare
vince-fugnitto
added
electron
There was a problem hiding this comment.
For the most part, this works as advertized. I did run into one oddity. It's possible to get into a state where running the electron app fails because electron-rebuild erroneously believes that native modules have been build for electron. Usually at startup, there's a message to the effect that some module did not self-register. When I was in that state and tried to run this script, I just got repeated logs about inability to analyze the performance. If there's some way to detect that error and provide a more helpful message, that might be useful.
cdamus
force-pushed
the
electron-startup
branch
from
4ff4f95 to
60f5cfd
Compare
|
Thanks, @colin-grant-work . Good points, all. I've pushed commit 60f5cfd that should address them, including adding a |
cdamus
force-pushed
the
electron-startup
branch
from
60f5cfd to
b682bdc
Compare
|
The force-push of commit edc22d5 just resolves the conflict in the change log. |
cdamus
force-pushed
the
electron-startup
branch
from
b682bdc to
edc22d5
Compare
|
@cdamus, please rebase this branch, and then I will merge it. |
- refactor a reusable module out of the startup measurement script - create a new script to measure Electron start-up performance - rename the measure-performance.js script to indicate that it measures the browser app specifically - add an --app/-a option to the extension-impact script to drive the Electron app as an alternative to the Browser app. Includes a bit of refactoring of this script and the app package template - incidentally fix the extension-impact.js stepping over itself trying to run the app while building it (was missing an await) Contributed on behalf of STMicroelectronics. Co-authored-by: Stefan Dirix <[email protected]> Signed-off-by: Christian W. Damus <[email protected]>
cdamus
force-pushed
the
electron-startup
branch
from
edc22d5 to
5242fbe
Compare
|
Thanks, @colin-grant-work I've force-pushed 5242fbe. The only conflict was in the changelog. |
What it does
Adds a new performance script for measurement of frontend start-up performance of the Electron example, similar to what was done in PR #9777. In fact, much of that script is factored out into a module of reusable functions to build the Electron script. This new script runs in much the same way, with similar command-line arguments to specify the number of runs and where to put the Chromium trace files.
Resolves #10440.
Example output:
Note that the existing NPM script for the browser start-up performance is renamed from
performance:startuptoperformance:startup:browserto distinguish it from the Electron case.A new command-line option
--app(-a) is added to theextension-impact.jsscript that selects the example application to build for execution of the tests. Supported values arebrowserandelectron; the default if not specified is the browser app as before.No new package dependencies are added.
Contributed on behalf of STMicroelectronics.
How to test
See the readme in the
scripts/performance/directory for details of how to run the script. For most cases, it is sufficient just to run the NPM script from the monorepo root:$ yarn performance:startup:electronThis will print out performance measurements with summary statistics as shown above. You may also import the traces generated by the script in the
scripts/performance/profiles/electron/directory into the Performance tab of the Chrome Developer Tools for analysis.Also, try running the Extension Impact script with the new option to drive the Electron app:
Note that executing the extension impact script on the Electron app after having run it on the browser app will often fail to get measurements for the
@theia/gitextension; it throws an exception about a mismatch in node versions in the native modules:This can be remedied by cleaning the Git workspace and building it again before running the script.
Review checklist
Reminder for reviewers